Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

send new-tip message to explorer #2903

Merged
merged 2 commits into from
Jan 25, 2021
Merged

send new-tip message to explorer #2903

merged 2 commits into from
Jan 25, 2021

Conversation

ecioppettini
Copy link
Contributor

remove the branch selection logic from the explorer by sending a message through the intercom from the branch selection function

this is similar to #2378 (there the new tip is sent through the websocket interface), so this helps a bit with the explorer extraction too.

@ecioppettini
send new-tip message to explorer
48ff2db 
so the logic will always be the same.
@ecioppettini ecioppettini added the A-explorer Area: Explorer API and backend label Jan 11, 2021
@ecioppettini ecioppettini requested a review from a team January 11, 2021 17:37
@ecioppettini ecioppettini self-assigned this Jan 11, 2021
eugene-babichenko
eugene-babichenko approved these changes Jan 13, 2021
View reviewed changes
Copy link
Contributor

@eugene-babichenko eugene-babichenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. BTW do we have any tests that verify the correctness of branch switching in the explorer?

@ecioppettini
Copy link
Contributor Author

LGTM. BTW do we have any tests that verify the correctness of branch switching in the explorer?

@dkijania it's probably better suited to answer that. As far as I remember, there was a non-functional test somewhere that checked the explorer tip against rest for 15 minutes, but I don't know if there's anything involving multiple nodes (for testing forks).

the explorer_sanity_test is probably enough to test that this works, though, as otherwise the transaction wouldn't be there, because the state would remain just as it was in the genesis.

@dkijania
Copy link
Contributor

@Enzoc4 @eugene-babichenko we have test in jormungandr-scenario-tests project: passive_node_explorer which basically check if single transaction send to leader node is reflected in explorer setup on passive node :

https://github.com/input-output-hk/jormungandr/blob/master/testing/jormungandr-scenario-tests/src/test/features/explorer.rs#L15

Please tell me if we need some another test

@ecioppettini
Copy link
Contributor Author

thanks @dkijania
I don't think we need anything else at least for this one

@mzabaluev mzabaluev merged commit 3e470c2 into master Jan 25, 2021
@mzabaluev mzabaluev deleted the send-new-tip-message-to-explorer branch January 25, 2021 12:16
@ecioppettini ecioppettini mentioned this pull request Jan 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eugene-babichenko eugene-babichenko eugene-babichenko approved these changes

Assignees

@ecioppettini ecioppettini

Labels
A-explorer Area: Explorer API and backend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ecioppettini @dkijania @eugene-babichenko @mzabaluev